standardise cmakelist #76
Conversation
LecrisUT
left a comment
LecrisUT
left a comment
There was a problem hiding this comment.
For more context for upstream, me and OP are Fedora packagers and we've encountered some peculiarities while trying to package this project.
The main change here is to install the Config.cmake file into the libdir folder, particularly because this is a compiled project.
CMakeLists.txt
Outdated
| target_compile_options(${PROJECT_NAME}_objlib PRIVATE $<$<CONFIG:RELEASE>:-s>) | ||
| # target_compile_options(${PROJECT_NAME}_objlib PRIVATE $<$<CONFIG:RELEASE>:-s>) | ||
| # try to make it optional. removed as it will break debuginfo generation. |
There was a problem hiding this comment.
I can't find documentation on the -s option, but overall I agree that all of these compile flags should be moved outside of the project configuration, since these affect downstream packagers. Use CMakePresets.json instead. If specific compile flags are indeed needed, please document them and guard them by the compiler ID/family.
For now though, we can circumvent this issue by specifying -DCMAKE_BUILD=RelWithDebInfo downstream, and minimize the changes of this PR a bit.
There was a problem hiding this comment.
-s Remove all symbol table and relocation information from the executable.
https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html
Let's split this chnage out for ease of reviewing.
CMakeLists.txt
Outdated
| target_compile_options(${PROJECT_NAME}_objlib PRIVATE $<$<CONFIG:RELEASE>:-s>) | ||
| # target_compile_options(${PROJECT_NAME}_objlib PRIVATE $<$<CONFIG:RELEASE>:-s>) | ||
| # try to make it optional. removed as it will break debuginfo generation. |
There was a problem hiding this comment.
-s Remove all symbol table and relocation information from the executable.
https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html
Let's split this chnage out for ease of reviewing.
ausyskin
left a comment
ausyskin
left a comment
There was a problem hiding this comment.
One small nit.
And can you format as one commit with "safestringlib: cmake:" header prefix, message body and Signed-off-by: line?
CMakeLists.txt
Outdated
| endif() | ||
|
|
||
| target_compile_options(${PROJECT_NAME}_objlib PRIVATE $<$<CONFIG:RELEASE>:-s>) | ||
| target_compile_options(${PROJECT_NAME}_objlib PRIVATE $<$<CONFIG:RELEASE>:-s>) |
There was a problem hiding this comment.
please remove stray whitespace
solomoncyj
force-pushed
the
patch-1
branch
2 times, most recently
from
db16981 to
5daa67b
Compare
ausyskin
left a comment
ausyskin
left a comment
There was a problem hiding this comment.
safestringlib: cmake: standardize cmakelist
Signed-off-by: yoong jin <solomonvyj@gmail.com>
3 participants
Hi, Fedora packager here. This pr is some quick suggestions for improvemnt in the cmakelists file